Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial submission #1

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Initial submission #1

merged 5 commits into from
Sep 23, 2024

Conversation

seratch
Copy link
Member

@seratch seratch commented Sep 19, 2024

Type of change (place an x in the [ ] that applies)

  • New sample
  • New feature
  • Bug fix
  • Documentation

Summary

Describe the goal of this PR. Mention any related Issue numbers.

If you are introducing a new sample app in this pull request, please also include a detailed description of the application’s purpose and functionality.

Requirements (place an x in each [ ] that applies)

  • I’ve checked my submission against the Samples Checklist to ensure it complies with all standards
  • I have ensured the changes I am contributing align with existing patterns and have tested and linted my code
  • I've read and agree to the Code of Conduct

@seratch seratch self-assigned this Sep 19, 2024
Copy link
Member Author

@seratch seratch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments for reviewers

@@ -1,3 +1,3 @@
[flake8]
max-line-length = 125
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For gen AI apps, this length is too small, thus I've changed it this time

.pytype/
.idea/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for PyCharm users

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't already have this in place in the main template, can we open a PR to update it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate you! 🙇🏼‍♀️

app.py Show resolved Hide resolved
app.event("assistant_thread_started")(start_thread_with_suggested_prompts)
app.event("assistant_thread_context_changed")(save_new_thread_context)
app.event("message", matchers=[is_user_message_event_in_assistant_thread])(respond_to_user_message)
app.event("message", matchers=[is_message_event_in_assistant_thread])(just_ack)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other message events such as message_changed, message_deleted etc. are produced by an assistant thread's behavior. Just acknowledging them.

app.event("message", matchers=[is_message_event_in_assistant_thread])(just_ack)


def is_message_event_in_assistant_thread(body: Dict[str, Any]) -> bool:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These is_xxx methods will be unnecessary once bolt-python's full support is released: slackapi/bolt-python#1162

@@ -0,0 +1,62 @@
from typing import Optional
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file will be unnecessary once bolt-python's full support is released: slackapi/bolt-python#1162

"scopes": {
"bot": [
"assistant:write",
"channels:join",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

channels:join, channels:history, groups:history are optional and they are necessary only for the channel summarization prompt pattern

Copy link
Contributor

@misscoded misscoded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for putting this together so quickly. 🙇🏼‍♀️

Since this isn't a true template (contains functionality and not just dummy text), have suggested changes to reflect that and make it a sample. Will also change the repository name to bolt-python-app-assistant!

.pytype/
.idea/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't already have this in place in the main template, can we open a PR to update it?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
app.py Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@seratch
Copy link
Member Author

seratch commented Sep 19, 2024

It's all set! Before we make this repo public (perhaps early next week), we can merge this PR.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@misscoded misscoded merged commit e243e7d into main Sep 23, 2024
3 checks passed
@seratch seratch deleted the initial-submission branch September 24, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants